Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More detailed stream states #894

Merged
merged 8 commits into from Dec 4, 2017
Merged

More detailed stream states #894

merged 8 commits into from Dec 4, 2017

Conversation

martinthomson
Copy link
Member

@martinthomson martinthomson commented Oct 20, 2017

This takes the state machines that @MikeBishop showed at the Seattle interim
and turns them into text and ASCII art.

I made a few changes in the process. The state where all data is available
didn't turn out to be any different to the state preceding it, so I removed it.
I also found it valuable to recognize that streams might be created for sending
before any frames are sent (I considered doing the same for receive streams,
but it complicated the transitions more than I liked, I could be convinced to
add this still, it just seems less valuable).

I added a section on composite states, showing how this mess all maps to stuff
you already know (h2).

This is still work-in-progress. I have changed the main chunk of text on
states, but I haven't changed any of the rest of the document, which still
talks about "idle" and all that. I'll do that next if people are generally
happy with the direction this is headed.

There are probably a few minor regressions here. In particular, I need to look at
what retransmissions can be received after they are no longer needed. But it's
late enough already.

Closes #634.

@martinthomson martinthomson added -transport editorial An issue that does not affect the design of the protocol; does not require consensus. labels Oct 20, 2017
@mikkelfj
Copy link
Contributor

Just a quick note from a glance: It woud be useful to have a clear overview of which frames can be sent from which states. In its current form self transitions are not included. It could be in a separate transition table to reduce clutter.

@martinthomson
Copy link
Member Author

This is now essentially complete - at least in my view. I've checked that I didn't miss anything from the original state machine and that the remainder of the document (and documents) don't refer to the old stream states.

An endpoint is expected to send STOP_SENDING frames if packets containing the
frame are lost. However, once either all stream data has been received or a
RST_STREAM frame - that is, any receive stream state other than "Recv" or "Size
Known" - sending the frame is not necessary.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be formulated more clearly? I think the paragraph is discussing retransmission while avoiding using the loaded term. E.g. what does "the frame is lost" mean? Which frame is lost? Does it mean something like: A STOP_SENDING frame should be retransmitted in case of packet loss until acknowledged or a FIN or RST_STREAM signal has been received.

Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a great improvement to the text. Some minor suggestions, but overall looks good.

same direction being counted as open.
A QUIC endpoint MUST NOT reuse a Stream ID. Open streams can be used in any
order. Streams that are used out of order result in lower-numbered streams in
the same direction being counted as open.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is both direction and type(uni/bidi). You use 'type' below, which seems fine here as well?

nit: "being opened." not being counted as opened.


Once all data has been either received or discarded, a sender is no longer
obligated to update the maximum received data for the connection.
Sending the first STREAM or STREAM_BLOCKED frame to be sent causes a send stream
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove "to be sent"

being in error.
In the "Send" state, an endpoint transmits - and retransmits as necessary - data
in STREAM frames. The endpoint respects the flow control limits of its peer,
accepting MAX_STREAM_DATA frames. It generates STREAM_BLOCKED frames if it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You talk about flow control a fair amount here. Is it worth adding a 'blocked' state to the state machine? I'm also happy with this as is and adding it later if we think it's useful. Just a thought.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd have to have it sit to the side of Send, though, with the ability to transition from Open to Blocked (no credit to start with) and the ability to oscillate between Blocked and Send, and the ability to close or reset from there as well. It feels like it would complicate the concept of the state machine much more than it would clarify.

Just say that, while in the Send state, you MUST obey flow control, sending STREAM_BLOCKED if you're stuck.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, blocked is a sub-state that doesn't really add much at this level of the description.

The "closed" state is the terminal state for a stream. Reordering might cause
frames to be received after closing, see {{state-hc-remote}}.
{{fig-stream-recv-states}} shows the states for the part of a stream that
receives data from a peer. The states for a receive stream mirror only some the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "only some of"

In the "Recv" state, the endpoint receives STREAM and STREAM_BLOCKED frames.
Incoming data is buffered and reassembled into the correct order for delivery to
the application. As data is consumed by the application and buffer space
becomes available, the endpoint sends MAX_STREAM_DATA frames to allow the peer
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MAY send?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, honestly - it's a MUST in a sense as well. Best to leave the normative language about that to other parts of the spec.

bidirectional stream initiated by the endpoint (type 0 for a client, type 1 for
a server) enters the "Open" state.

A bidirectional stream also opens when a MAX_STREAM_DATA frame is received.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would move this up a paragraph so it's next to the other frames.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added it to the list and added a parenthetical, which is ugly, but breaks the loop of dependencies.

An endpoint is expected to send STOP_SENDING frames if packets containing the
frame are lost. However, once either all stream data has been received or a
RST_STREAM frame - that is, any receive stream state other than "Recv" or "Size
Known" - sending the frame is not necessary.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor nit: This sentence doesn't parse for me.

However, once either all stream data has been received or a RST_STREAM frame sending the frame is not necessary.

Is the idea as below?

An endpoint is expected to only retransmit STOP_SENDING frames for receive stream states "Recv" and "Size Known". For example, if all stream data, or a RST_STREAM frame, has been received, retransmitting STOP_SENDING frames is not necessary.

Copy link
Contributor

@MikeBishop MikeBishop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if it's time to separate frames into those sent on/about send streams, on/about receive streams, or on the connection. It's been there implicitly, but this change makes it even more glaring.

An endpoint MUST NOT send a STREAM or RST_STREAM frame for a stream ID that is
higher than the peers advertised maximum stream ID (see
{{frame-max-stream-id}}).
This section describes the two types of QUIC stream in terms of the states that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"states of", I think. Otherwise, this is hard to parse.

being in error.
In the "Send" state, an endpoint transmits - and retransmits as necessary - data
in STREAM frames. The endpoint respects the flow control limits of its peer,
accepting MAX_STREAM_DATA frames. It generates STREAM_BLOCKED frames if it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd have to have it sit to the side of Send, though, with the ability to transition from Open to Blocked (no credit to start with) and the ability to oscillate between Blocked and Send, and the ability to close or reset from there as well. It feels like it would complicate the concept of the state machine much more than it would clarify.

Just say that, while in the Send state, you MUST obey flow control, sending STREAM_BLOCKED if you're stuck.

that it wishes to abandon transmission of stream data. Similarly, the endpoint
might receive a STOP_SENDING frame from its peer. This causes the endpoint to
send a RST_STREAM frame, which causes the stream to enter the "Reset Sent"
state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pedant alert: The wording here doesn't make it blatantly obvious that "signal ... abandon" is done by RST_STREAM. Maybe replace "This causes the endpoint to send..." with "In either case, the endpoint sends..."?

all data. This includes sending all data carried in STREAM frames including
the terminal STREAM frame that contains a FIN flag.
An endpoint MAY send a RST_STREAM to open a send stream; this causes the send
stream to open and immediately transition to the "Reset Sent" state.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the stream was in the "Open" state before you send anything, describing the RST as opening the stream feels a little awkward. Maybe say "...as the first frame on a send stream. This causes the send stream to immediately transition...".


A stream also becomes "closed" when the endpoint sends a RST_STREAM frame.
Once a packet containing a RST_STREAM has been successfully acknowledged, the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: From the receiver's side of the ACK, there are no unsuccessful acknowledgements.

states of the send stream at the peer. A receive stream doesn't track states on
the send stream that can't be observed, such as the "Open" state; instead,
receive streams track the delivery of data to the application or application
protocol.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...some of which cannot be observed by the sender.

previously-sent STREAM frames.
~~~
o
| Recv STREAM / STREAM_BLOCKED / RST_STREAM
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes it look like two RST_STREAMs are needed to enter "RST received."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. But I don't like having two entry points to the state machine, that's worse and the prose is clear enough. Note also here that STREAM+FIN has the exact same effect.

| Recv STREAM / STREAM_BLOCKED / RST_STREAM
| Open Paired Stream (bidirectional)
| Recv MAX_STREAM_DATA
v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the send side, you have separate states for "I know the stream is open" and "data is flowing." On the receive side, you go immediately to the "Recv" state. Is there value in splitting those?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, and even drew it up, but it makes the states harder, not easier to describe. The Open state is convenient as a way to discuss some of the state transitions when sending, but I couldn't find an analogous benefit on the receive side and only complications to the state machine. The only advantage would be in describing the receive side of a bidirectional stream, and that didn't really need an extra state.

Also, it would make the two RST_STREAM frames problem a three RST_STREAM frames problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so?

to retransmit a lost STOP_SENDING frame if the stream has already been closed
in the appropriate direction since the frame was first generated.
See {{packetization}}.
An endpoint is expected to send STOP_SENDING frames if packets containing the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send => resend, send another, etc.

in the appropriate direction since the frame was first generated.
See {{packetization}}.
An endpoint is expected to send STOP_SENDING frames if packets containing the
frame are lost. However, once either all stream data has been received or a
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"all stream data or a RST_STREAM has been received"

@ianswett ianswett mentioned this pull request Nov 11, 2017
Copy link
Contributor

@ianswett ianswett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One comment/question, but looks good otherwise.

in sequential order. Open streams can be used in any order. Streams
that are used out of order result in lower-numbered streams in the
same direction being counted as open.
A QUIC endpoint MUST NOT reuse a Stream ID. Open streams can be used in any
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be Streams MUST be created in sequential order. I think there's an open issue just for that, so I'd rather keep that as is in this PR, unless it's somehow necessary to change it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot that I did that. Since it's unenforceable, can't we just concede that open-in-order isn't strictly an interoperability requirement? I could restore text with weasel words, but I've been convinced that strictly legislating here isn't constructive.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough, make sure to find and close the issue this was discussed in then.

| Recv STREAM / STREAM_BLOCKED / RST_STREAM
| Open Paired Stream (bidirectional)
| Recv MAX_STREAM_DATA
v
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so?

Copy link
Member Author

@martinthomson martinthomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Ugh, power cut yesterday dropped my comments...)

previously-sent STREAM frames.
~~~
o
| Recv STREAM / STREAM_BLOCKED / RST_STREAM
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know. But I don't like having two entry points to the state machine, that's worse and the prose is clear enough. Note also here that STREAM+FIN has the exact same effect.

| Recv STREAM / STREAM_BLOCKED / RST_STREAM
| Open Paired Stream (bidirectional)
| Recv MAX_STREAM_DATA
v
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered that, and even drew it up, but it makes the states harder, not easier to describe. The Open state is convenient as a way to discuss some of the state transitions when sending, but I couldn't find an analogous benefit on the receive side and only complications to the state machine. The only advantage would be in describing the receive side of a bidirectional stream, and that didn't really need an extra state.

Also, it would make the two RST_STREAM frames problem a three RST_STREAM frames problem.

bidirectional stream initiated by the endpoint (type 0 for a client, type 1 for
a server) enters the "Open" state.

A bidirectional stream also opens when a MAX_STREAM_DATA frame is received.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added it to the list and added a parenthetical, which is ugly, but breaks the loop of dependencies.

in sequential order. Open streams can be used in any order. Streams
that are used out of order result in lower-numbered streams in the
same direction being counted as open.
A QUIC endpoint MUST NOT reuse a Stream ID. Open streams can be used in any
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot that I did that. Since it's unenforceable, can't we just concede that open-in-order isn't strictly an interoperability requirement? I could restore text with weasel words, but I've been convinced that strictly legislating here isn't constructive.

This takes the state machines that @MikeBishop showed at the Seattle interim
and turns them into text and ASCII art.

I made a few changes in the process.  The state where all data is available
didn't turn out to be any different to the state preceding it, so I removed it.
I also found it valuable to recognize that streams might be created for sending
before any frames are sent (I considered doing the same for receive streams,
but it complicated the transitions more than I liked, I could be convinced to
add this still, it just seems less valuable).

I added a section on composite states, showing how this mess all maps to stuff
you already know (h2).

This is still work-in-progress.  I have changed the main chunk of text on
states, but I haven't changed any of the rest of the document, which still
talks about "idle" and all that.  I'll do that next if people are generally
happy with the direction this is headed.

Closes #634.
@martinthomson
Copy link
Member Author

@MikeBishop, what do you think is remaining here? I'm happy to consider splitting frames into two categories, but that's something I'd prefer to do as a separate change.

@martinthomson martinthomson removed the editorial An issue that does not affect the design of the protocol; does not require consensus. label Dec 4, 2017
@martinthomson martinthomson merged commit 272cab7 into master Dec 4, 2017
@martinthomson martinthomson deleted the stream-states-mk8 branch December 4, 2017 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants